fix: only append init segment on change #1128
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1128 +/- ##
=======================================
Coverage 86.19% 86.20%
=======================================
Files 39 39
Lines 9281 9284 +3
Branches 2126 2126
=======================================
+ Hits 8000 8003 +3
Misses 1281 1281
Continue to review full report at Codecov.
|
| @@ -3142,6 +3142,7 @@ QUnit.module('SegmentLoader', function(hooks) { | |||
| appends[1].initSegment, | |||
| 'appended a different init segment' | |||
| ); | |||
| loader.appendInitSegment_.audio = true; | |||
There was a problem hiding this comment.
These tests were relying on re-appending init segments, which shoudn't happen here, but I think these tests are still valuable.
There was a problem hiding this comment.
It's probably worth adding those details to the comment (here and below) to ensure we don't forget why we force it in the future.
src/segment-loader.js
Outdated
| // we should only be appending the next init segment if we detect a change, or if | ||
| // the segment has a map | ||
| this.appendInitSegment_[type] = map ? true : false; | ||
| this.appendInitSegment_[type] = false; |
There was a problem hiding this comment.
Maybe we should update the comment to something like: "disable future init segments appends for this type (until a change is necessary)"
| this.appendInitSegment_ = { | ||
| video: true, | ||
| audio: true | ||
| }; |
There was a problem hiding this comment.
Should this be dependent on the type that this loader is handling?
There was a problem hiding this comment.
I don't think it should because if we get an audio or video init segment on this segment loader, we should re-append it. It won't likely happen for a video only loader or an audio only loader, but at this point we won't actually know what type of content we are requesting, as we get that in trackinfo.
| @@ -3142,6 +3142,7 @@ QUnit.module('SegmentLoader', function(hooks) { | |||
| appends[1].initSegment, | |||
| 'appended a different init segment' | |||
| ); | |||
| loader.appendInitSegment_.audio = true; | |||
There was a problem hiding this comment.
It's probably worth adding those details to the comment (here and below) to ensure we don't forget why we force it in the future.
Co-authored-by: Garrett Singer <[email protected]>
Description
Only append init segments on change.